Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vim: Make syntax colors more like solarized #70

Merged
merged 2 commits into from
Mar 22, 2020
Merged

Conversation

jan-warchol
Copy link
Owner

Since Selenized ANSI palette doesn't have orange, I'm using magenta for
PreProc. Also, I strongly dislike Solarized's green keywords - I think
they are ugly and not very readable - so I use bold foreground for that.

image
from left to right: old selenized, new selenized, solarized

One other thing I'm considering is differentiating numbers and booleans
from strings in some way, maybe using green?

@jan-warchol
Copy link
Owner Author

@fladd Please review :)
You can find the code I've used for testing here: https://github.com/jan-warchol/highlighting-code-samples

@fladd
Copy link

fladd commented Mar 15, 2020

@jan-warchol Certainly an improvement.

However, some thoughts:

  • I think keywords are not visible enough in bold white. I really think they need a colour. When I started using Solarized I was also not a big fan of the green, but it grew on me over time. Before I was also used to dark orange for keywords (like the default IDLE colouring).
  • Magenta for import colours works for me, but aren't the {} in strings the same colour as import statement?
  • Differentiating numbers and booleans from strings is possible I guess, although booleans are built-in constants (like e.g. None; but then again, technically, booleans are a subtype of integers...)
  • You seem to be using a different Solarized than I am. For me object for instance is blue, not orange, which also makes more sense I guess, since it is a built-in.

@fladd
Copy link

fladd commented Mar 15, 2020

@jan-warchol
Copy link
Owner Author

jan-warchol commented Mar 15, 2020 via email

@jan-warchol
Copy link
Owner Author

@fladd
Copy link

fladd commented Mar 15, 2020

What about yellow? I would be ok with yellow keywords (either bold or not), could you try it? (I'm afk)

The problem there is that it will collide with Type. But for people who don't map classes to types this might work.
image

Another alternative could be magenta:
image

But then again, my main point was that people might simply expect Selenized to follow the same colouring scheme as Solarized (which Selenized says it improves on). The more it now diverts from Solarized again, the less it can be used as a drop-in replacement.

You mean in original solarized? One is categorized as PreProc (orange), another as Special (red). However, red and orange in solarized are so similar that you could have confused them (one of the reasons for creating selenized).

I see. So making PreProc red would also be an option?

I'm using the one from official repo. However, are you using a language pack? Edit: sorry, I replied to email from my phone and formatting was mangled.

I am using Solarized8, because the original Solarized has a bug that breaks loading other colour schemes after it (which Solarized8 fixes).

@fladd
Copy link

fladd commented Mar 15, 2020

@dngray
Copy link
Contributor

dngray commented Mar 16, 2020

I really like the logic used in the DESIGN.md for your limestone theme! I like that the operators and magic numbers are easier to see.

I rather like the middle example from #70 (comment) except for the operators/magic numbers kinda blend in.

@dngray
Copy link
Contributor

dngray commented Mar 16, 2020

I also had another thought about this and I don't really think solarized has any methodology on the token coloring. I am not opposed to making improvements. I do not think we should be afraid to improve on solarized in this way.

Looking at the magenta version in #70 (comment) I can't say I am thrilled about import and from being the same color as keywords like def, try, except, return.

@fladd
Copy link

fladd commented Mar 16, 2020

I find green for Type problematic, since it does not stick out enough due to being too similar to (potentially surrounding) strings.

Have you already tried sticking to green for the keywords (like Solarized), but making them bold?

@jan-warchol
Copy link
Owner Author

What about yellow? I would be ok with yellow keywords (either bold or not), could you try it? (I'm afk)

The problem there is that it will collide with Type. But for people who don't map classes to types this might work.

I definitely prefer yellow for keywords. I'm 99% sure that no combination could be better, and that it's an improvement over original solarized. The collision with Type styling can be avoided by simply swapping the colors - yellow and green are next to each other on colorwheel so the result has a similar feel to original solarized:

image

image

Interestingly, solarized colored vimscript a little different so the result is almost the same:

image

But then again, my main point was that people might simply expect Selenized to follow the same colouring scheme as Solarized (which Selenized says it improves on). The more it now diverts from Solarized again, the less it can be used as a drop-in replacement.

I understand. At the same time, many themes named "solarized" are not consistent with each other. So, my question is: would the proposed version be good enough that you use it?

You mean in original solarized? One is categorized as PreProc (orange), another as Special (red). However, red and orange in solarized are so similar that you could have confused them (one of the reasons for creating selenized).

I see. So making PreProc red would also be an option?

Hmm. I don't think it'd be good it both PreProc and Special had the same color. I tried it and IMO it's not good, too many things have the same color:

image

I am using Solarized8, because the original Solarized has a bug that breaks loading other colour schemes after it (which Solarized8 fixes).

I see. Do you want me to check with Solarized8?

@fladd
Copy link

fladd commented Mar 16, 2020

Your first image is a good example that switching yellow and green leads to inferior readability. Look how the AttributeError pops out in Solarized, but becomes almost completely invisible between all the blue and turquoise in Selenized. With keywords being most often not right within the text, but at the beginning, they are still well readable in green.

@jan-warchol
Copy link
Owner Author

jan-warchol commented Mar 16, 2020

Have you already tried sticking to green for the keywords (like Solarized), but making them bold?

I tried it now and I must say I don't like the result at all :/

Your first image is a good example that switching yellow and green leads to inferior readability. Look how the AttributeError pops out in Solarized, but becomes almost completely invisible between all the blue and turquoise in Selenized.

Okay, you have a point here. 🤔 On the other hand, vimscript doesn't have this problem - green elements are nicely visible...
🤔
What about using orange? Orange would pop. Of course it won't work with ANSI color codes, but many terminals support true color nowadays. (alternatively, we could then make PreProc orange - as in the original - and use magenta for Type.)

Anyway, I need to sleep over it. I really, really dislike green keywords. (Although I would use them if it turns out that the majority of uses want them badly.)

@jan-warchol jan-warchol force-pushed the more-solarized branch 2 times, most recently from 6c4c857 to 8549683 Compare March 22, 2020 07:35
Since Selenized ANSI palette doesn't have orange, I'm using magenta for
PreProc. Also, I strongly dislike Solarized's green keywords - I think
they are ugly and not very readable - so I use yellow for that. Since
this choice was strongly opposed by @fladd, I've added an option to
override it.

Closes issue #69.
@jan-warchol
Copy link
Owner Author

jan-warchol commented Mar 22, 2020

Okay, I slept over it and my decision is to have a "solarized compatibility" switch - see updated readme.

I've also looked at samples in other languages and the problem with green not being visible enough seems not to appear there:

image

In fact, I actually like the result in that the types, which are sort of boilerplate, are not too strong.

As for exceptions in Python, I agree they look bad, and I'm going to address it in #68.

Is this ok with you, @fladd? I'd like to merge this PR and move on :).

@fladd
Copy link

fladd commented Mar 22, 2020

@jan-warchol If you add some string to the Java example, then the problem will be the same.

In theory, having this switch is fine for me.

More generally, linking Python exceptions to Type is the correct thing to do, so I would not advice to change this and over-complicate things by extensively switching things around for different languages. Also on the background that Selenized has been suggested to be included in Vim in this thread (which is how I came here to begin with), and anything but a clean and simple color scheme will not make the inclusion list.

@jan-warchol
Copy link
Owner Author

jan-warchol commented Mar 22, 2020

More generally, linking Python exceptions to Type is the correct thing to do, so I would not advice to change this and over-complicate things by extensively switching things around for different languages. Also on the background that Selenized has been suggested to be included in Vim in this thread (which is how I came here to begin with), and anything but a clean and simple color scheme will not make the inclusion list.

I see your point. I also don't want to over-complicate things (even though we may differ as to the point at which over-complication starts).

@jan-warchol If you add some string to the Java example, then the problem will be the same.

In theory, having this switch is fine for me.

Ok, I'm going to merge this PR then - but I don't consider the issue settled forever. In particular, I'm going to expand the GUI version to include orange and violet, and this may change some things, so I'd be very interested in your feedback.

Also, I may make the green keywords the default version if more people will request it.

@jan-warchol
Copy link
Owner Author

Also, I may make the green keywords the default version if more people will request it.

See #74. (@fladd @dngray please vote there. If the issue gets at least 10 upvotes and no more than 8 downvotes I'll make the switch.)

@fladd
Copy link

fladd commented Apr 23, 2020

Since Selenized ANSI palette doesn't have orange[...]

@jan-warchol Why is that actually? According to this, Selenized has the same colours as Solarized, including orange:
https://camo.githubusercontent.com/566ad19fe03c0fa8e48d619868378e5e3542b497/68747470733a2f2f692e696d6775722e636f6d2f314173433370532e706e67

@jan-warchol
Copy link
Owner Author

Since Selenized ANSI palette doesn't have orange[...]

@jan-warchol Why is that actually? According to this, Selenized has the same colours as Solarized, including orange:
https://camo.githubusercontent.com/566ad19fe03c0fa8e48d619868378e5e3542b497/68747470733a2f2f692e696d6775722e636f6d2f314173433370532e706e67

Hi, sorry for late reply.

Selenized itself does have orange, but ANSI standard doesn't have a color code for orange - it defines 8 colors (plus 8 bright versions), but these include "black" and "white": see https://en.wikipedia.org/wiki/ANSI_escape_code#Colors So there is no way to have orange while complying with the standard meaning of ANSI codes.

Now, Solarized works around this by assigning ANSI codes non-standard meanings, but I don't want to do this because it leads to all sorts of weird output. There's an image in the docs that I think you'll find helpful:
https://github.com/jan-warchol/selenized/blob/master/whats-wrong-with-solarized.md#better-terminal-compatibility

Is it clearer now?

@fladd
Copy link

fladd commented Aug 5, 2020

@jan-warchol It's clearer now for the terminal themes, but not necessarily for the Vim theme, which could use the full palette, right.

clach04 pushed a commit to clach04/selenized that referenced this pull request Dec 14, 2024
Vim: Make syntax colors more like solarized
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants